[llvm-profgen] Set FirstLoadableAddress only once #151257
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-pgo Author: Wei Wang (apolloww) ChangesThe address of the first loadable segment can be zero, so Full diff: https://github.com/llvm/llvm-project/pull/151257.diff 1 Files Affected:
diff --git a/llvm/tools/llvm-profgen/ProfiledBinary.h b/llvm/tools/llvm-profgen/ProfiledBinary.h
index 0588cb48b2af6..8bc7e7b3b0061 100644
--- a/llvm/tools/llvm-profgen/ProfiledBinary.h
+++ b/llvm/tools/llvm-profgen/ProfiledBinary.h
@@ -199,7 +199,7 @@ class ProfiledBinary {
// The runtime base address that the first executable segment is loaded at.
uint64_t BaseAddress = 0;
// The runtime base address that the first loadabe segment is loaded at.
- uint64_t FirstLoadableAddress = 0;
+ std::optional<uint64_t> FirstLoadableAddress;
// The preferred load address of each executable segment.
std::vector<uint64_t> PreferredTextSegmentAddresses;
// The file offset of each executable segment.
@@ -381,7 +381,10 @@ class ProfiledBinary {
return PreferredTextSegmentAddresses[0];
}
// Return the preferred load address for the first loadable segment.
- uint64_t getFirstLoadableAddress() const { return FirstLoadableAddress; }
+ uint64_t getFirstLoadableAddress() const {
+ assert(FirstLoadableAddress && "FirstLoadableAddress must be set.");
+ return *FirstLoadableAddress;
+ }
// Return the file offset for the first executable segment.
uint64_t getTextSegmentOffset() const { return TextSegmentOffsets[0]; }
const std::vector<uint64_t> &getPreferredTextSegmentAddresses() const {
|
|
I think the convention is to leave the address zero unmapped (ie. at least the first page) so that null pointer dereferences cause segmentation faults. Can you describe the situation where you have seen this behaviour? |
apolloww
force-pushed
the
profgen_fix_firstloadableaddr
branch
from
c389225 to
6b8ca0f
Compare
I saw The binary has first |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The address of the first loadable segment can be zero, so
FirstLoadableAddressmay actually point to the second loadable segment. Use an optional instead.